Skip to content

Conversation

@AlbertLucianto
Copy link

Add assertion when a route component is null or undefined, a fix for issue #2038.

Really appreciate for a review.

Copy link
Member

@posva posva left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified the condition but you still need to add more checks because, the component option can actually be absent if other options are present (like redirect)

@AlbertLucianto
Copy link
Author

Oh sorry I missed that. Will try to work on that.

@posva
Copy link
Member

posva commented Feb 2, 2018

Keep in mind you can run the tests locally too, it will save you some time 😄

@AlbertLucianto
Copy link
Author

Just now, I've tried to trace through what might be throwing error when it shouldn't. Then I found in Route Matching that the routes does not have any components nor any other options like redirections, aliases. So I think it is probably better to just warn instead of asserting error.

You might want to review it again. Anyway thanks for the guide.

maps = createRouteMap([{ path: '/' }])
expect(console.warn).toHaveBeenCalled()
})

Copy link

@vinibrsl vinibrsl Feb 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would create an spec similar to this one, but to test that this is not affecting production env, with not.toHaveBeenCalled 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request WIP work in progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants